-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Alternative fix of #60182 #60298
Alternative fix of #60182 #60298
Conversation
Tagging subscribers to this area: @mangod9 |
Hi @sakno, could you please rebase to main and once CI completes we could get this merged. Thx. |
@mangod9 , done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
@BruceForstall formatting failed https://dev.azure.com/dnceng/9ee6d478-d288-47f7-aacc-f6e6d082ae6d/_apis/build/builds/1459736/logs/217 I'll merge as this didn't touch the JIT code (should formatting only trigger if the JIT tree is changed?) |
@danmoseley It looks like the JIT formatting job runs anytime there's a change in coreclr, and that appears to include System.Private.CoreLib: https://dev.azure.com/dnceng/public/_build/results?buildId=1459736&view=logs&jobId=db5b9618-eefc-58e8-4554-fbce452d4dac&j=c8204876-824e-5bf9-8c45-a4628bfcec7d&t=c0cab5ad-9de6-5568-2ae4-6c24d2f3402f @safern can probably explain this better. Maybe we could add more granularity there to allow only running if there's a change in the JIT directory itself. As for the failure, I did briefly break the job last week (fixed with #61334), but it's not clear if this change was unlucky to pick up the break, or hit some other unknown error. |
what folders should it trigger on exactly? then we can create a new group here
and then use it here runtime/eng/pipelines/runtime.yml Line 197 in 03e90a5
|
@danmoseley Something like: #61632? (can move discussion there) |
Alternative way to fix #60182 without try-catch block. Quick fix is available as separated PR #60224